Conversation
55b9269 to
02df632
Compare
f83ac21 to
f92ea6b
Compare
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
… page Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Replace AI-generated bullet lists and abstract stubs with a walkthrough of the two-phase recording system. Use the real ObjectMovedRateMetric implementation as the custom metric example. Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Move concept_policy_design, concept_remote_policies_design, and policy_evaluation/evaluation_types into a new concepts/policy/ subdirectory. Remove the standalone Policy Evaluation section from the top-level navigation. Update all cross-references. Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Resolve conflict in docs/index.rst: keep Concepts toctree from branch, add new Arena in Your Repo section from main. Move Arena in Your Repo above Concepts in the navigation. Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Co-authored-by: isaaclab-review-bot[bot] <270793704+isaaclab-review-bot[bot]@users.noreply.github.com>
Co-authored-by: isaaclab-review-bot[bot] <270793704+isaaclab-review-bot[bot]@users.noreply.github.com>
Co-authored-by: isaaclab-review-bot[bot] <270793704+isaaclab-review-bot[bot]@users.noreply.github.com>
Replace nonexistent how_open() with get_openness() to match the actual Openable affordance API. Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Co-authored-by: isaaclab-review-bot[bot] <270793704+isaaclab-review-bot[bot]@users.noreply.github.com>
Co-authored-by: isaaclab-review-bot[bot] <270793704+isaaclab-review-bot[bot]@users.noreply.github.com>
kellyguo11
left a comment
There was a problem hiding this comment.
Nice work cleaning up the docs — the rewritten pages are a massive improvement over the old templated format. The restructuring into scene/, task/, embodiment/, and policy/ subdirectories makes the concept docs much more navigable.
A few things I spotted:
Reset submodules/IsaacLab back to the commit on main (e57379c6). It was inadvertently changed in b4311e5. Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Superpowers plans and specs are now stored outside the repo, so the gitignore entry is no longer needed. Signed-off-by: Clemens Volk <cvolk@nvidia.com>
Signed-off-by: Clemens Volk <cvolk@nvidia.com>
RST requires a blank line before bullet lists for correct parsing. Signed-off-by: Clemens Volk <cvolk@nvidia.com>
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR restructures the concepts documentation from a flat directory into a topical hierarchy (scene/, task/, embodiment/, policy/), rewrites the content to be clearer and more concise, and adds helpful new diagrams. The stated goal — eliminating AI-generated prose — is achieved well; the new text reads naturally and focuses on what the reader needs to know.
Design Assessment
Design is sound. The reorganization into subdirectories that mirror the Arena architecture (scene, task, embodiment, policy) is a good structural match. The new index.rst pages for each sub-section provide high-level orientation before drilling into details, which is a significant readability improvement over the previous flat list.
Findings
🔵 Suggestion: show_nav_level change from 2 to 1 — docs/conf.py
Changing show_nav_level from 2 to 1 means the sidebar navigation will show one fewer level of depth by default. This makes sense with the new subdirectory structure (where the sub-section index.rst files act as the second level), but it's worth verifying the sidebar feels right once the docs are deployed — with the deeper hierarchy, users might benefit from show_nav_level: 2 to see the leaf pages without clicking through.
🔵 Suggestion: Concept overview no longer links to sub-section pages — docs/pages/concepts/concept_overview.rst
The old overview had explicit :doc: links to each concept page (e.g., "See :doc:concept_scene_design for more details"). The new version relies on the reader finding sub-sections via the sidebar or toctree. This is fine for the restructured layout, but consider adding a brief sentence pointing to the sub-section pages (e.g., "The following pages describe each component in detail") to help users who land on the overview page directly.
🔵 Suggestion: Placeable affordance mentioned but not explained — docs/pages/concepts/scene/index.rst
Line 26 mentions Placeable alongside Openable as an example affordance, but the affordances page only documents Openable and Pressable. Either add a brief note about Placeable on the affordances page, or use Pressable in the scene index instead for consistency.
Test Coverage
No tests needed — this is a pure documentation change with no runtime code modifications.
CI Status
- ✅ Build the docs (pre-merge): passed
- ✅ Pre-commit: passed
- 🔄 Run tests: in progress (not relevant for a docs-only change)
- 🔄 Run policy-related tests with GR00T: in progress (not relevant for a docs-only change)
Verdict
APPROVE
Clean restructuring with no broken cross-references. The new docs are clearer, better organized, and notably less verbose than what they replace. The suggestions above are minor polish items — nothing blocking. Good work @alexmillane.
|
|
||
| Assets are loaded from the asset registry by name. An asset can be a | ||
| background, a rigid object, or a set of objects (``RigidObjectSet``). | ||
| Assets can also carry affordances (e.g. ``Openable``, ``Placeable``) that |
There was a problem hiding this comment.
Nit: Placeable isn't documented on the affordances page — it only covers Openable and Pressable. Consider changing this to Openable, ``Pressable``` for consistency, or adding a brief mention of Placeable to the affordances page.
Summary
Rework the concepts documentation to eliminate AI slop.